Skip to content

Changes going from 77bc2457cc76 (2.75.2) to 3f8b98c2db70 (2.74.1)#545

Draft
ernestl wants to merge 31 commits intoreview-head-77bc2457from
review-base-3f8b98c2
Draft

Changes going from 77bc2457cc76 (2.75.2) to 3f8b98c2db70 (2.74.1)#545
ernestl wants to merge 31 commits intoreview-head-77bc2457from
review-base-3f8b98c2

Conversation

@ernestl
Copy link
Copy Markdown
Member

@ernestl ernestl commented Apr 30, 2026

No description provided.

chrisccoulson and others added 30 commits February 18, 2026 19:40
The preinstall checks currently require a system to be in deployed mode
(for UEFI versions >= 2.5). Relax this to allow an opt-in to user mode
for systems that run UEFI versions >= 2.5 but where the firmware
settings don't permit enabling deployed mode.

To support this, a new WithSecureBootUserMode option is added for
AddPCRProfile. If this option is supplied on a system that is in user
mode, 2 branches will be created. One of these will include the user
mode related measurements and the other branch will be for deployed
mode, which allows a system to transition from user mode to deployed
mode without requiring a recovery key.

This is an opt-in rather than automatic to avoid the scenario where a
system is initially in deployed mode but a later firmware configuration
change reverts to user mode, the user enters their recovery key on the
next boot and then snapd automatically repairs with a PCR profile that
includes the newly degraded setting. In this case, we want the user to
explicitly opt-in to this as an acknowledgement that the firmware
configuration has been changed.

Fixes: FR-12184
Fixes: #502
This adds initial APIs for resetting the TPM's dictionary attack counter
to replace the direct use of go-tpm2's
TPMContext.DictionaryAttackLockReset API.

This doesn't yet include the changes to Connection.EnsureProvisioned
which will create the authorization data that will be persisted by
snapd and passed to this new API - this is coming in a follow-up PR.

This is split into 2 APIs:
- Connection.ResetDictionaryAttackLock which will be supplied with data
  that will be provided by an updated EnsureProvisioned API.
- Connection.ResetDictionaryAttackLockWithAuthValue which will be
  supplied with a plain authorization value, previously chosen by snapd
  and provided to the current EnsureProvisioned API.

I did think about having this as 1 API, and handling the case where
decoding of the authorization data fails and treating it as an
authorization value if the length is 32 bytes. But as this value is
chosen by snapd and nothing in secboot enforces a size of 32 bytes, I
thought it was safer to have this as 2 APIs so that snapd can be
explicit about the format of the supplied data.

Fixes: FR-10809
This function shouldn't fail on unexpected event types. Instead, the
unexpected event types should trigger errors later on, resulting in more
appropriate error messages.

Fixes: FR-12830
Fixes: #514
This refines the checks that run on Intel systems so that appropriate
error messages are returned if the system is in manufacturing mode. The
changes are based on checks in the coreboot repository, specifically
https://github.com/coreboot/coreboot/blob/eb5bdf06b92534b6f66f612297a4ccb69008b4ac/src/soc/intel/common/block/cse/cse_spec.c#L15

Fixes: FR-12761
Fixes: #515
Based on the understanding that the ASP still acts as the root-of-trust
for measurement when PSB is disabled, permit platforms with an AMD CPU
when PSB is disabled. In this case, as firmware integrity is provided by
measured boot, we require profiles to be locked to PCR0 which is not
necessary when PSB is enabled.
tpm2: Add initial Connection.ResetDictionaryAttackLock APIs.

This adds initial APIs for resetting the TPM's dictionary attack counter
to replace the direct use of go-tpm2's
`TPMContext.DictionaryAttackLockReset` API.

This doesn't yet include the changes to `Connection.EnsureProvisioned`
which will create the authorization data that will be persisted by
snapd and passed to this new API - this is coming in a follow-up PR.

This is split into 2 APIs:
- `Connection.ResetDictionaryAttackLock` which will be supplied with data
  that will be provided by an updated `EnsureProvisioned` API.
- `Connection.ResetDictionaryAttackLockWithAuthValue` which will be
  supplied with a plain authorization value, previously chosen by snapd
  and provided to the current `EnsureProvisioned` API.

I did think about having this as 1 API, and handling the case where
decoding of the authorization data fails and treating it as an
authorization value if the length is 32 bytes. But as this value is
chosen by snapd and nothing in secboot enforces a size of 32 bytes, I
thought it was safer to have this as 2 APIs so that snapd can be
explicit about the format of the supplied data.

Fixes: FR-10809
…-optin

efi, efi/preinstall: Allow opt-in to secure boot being in user mode.

The preinstall checks currently require a system to be in deployed mode
(for UEFI versions >= 2.5). Relax this to allow an opt-in to user mode
for systems that run UEFI versions >= 2.5 but where the firmware
settings don't permit enabling deployed mode.

To support this, a new `WithSecureBootUserMode` option is added for
`AddPCRProfile`. If this option is supplied on a system that is in user
mode, 2 branches will be created. One of these will include the user
mode related measurements and the other branch will be for deployed
mode, which allows a system to transition from user mode to deployed
mode without requiring a recovery key.

This is an opt-in rather than automatic to avoid the scenario where a
system is initially in deployed mode but a later firmware configuration
change reverts to user mode, the user enters their recovery key on the
next boot and then snapd automatically repairs with a PCR profile that
includes the newly degraded setting. In this case, we want the user to
explicitly opt-in to this as an acknowledgement that the firmware
configuration has been changed.

Fixes: FR-12184
Fixes: #502
…nufacturing-mode-check

Preinstall improve csme manufacturing mode check.

This refines the checks that run on Intel systems so that appropriate
error messages are returned if the system is in manufacturing mode. The
changes are based on checks in the coreboot repository, specifically
https://github.com/coreboot/coreboot/blob/eb5bdf06b92534b6f66f612297a4ccb69008b4ac/src/soc/intel/common/block/cse/cse_spec.c#L15

Fixes: FR-12761
Fixes: #515
efi/preinstall: relax checkSecureBootPolicyForDegradedFirmwareSettings.

This function shouldn't fail on unexpected event types. Instead, the
unexpected event types should trigger errors later on, resulting in more
appropriate error messages.

Fixes: FR-12830
Fixes: #514
efi/preinstall: Don't require Platform Secure Boot on AMD.

Based on the understanding that the ASP still acts as the root-of-trust
for measurement when PSB is disabled, permit platforms with an AMD CPU
when PSB is disabled. In this case, as firmware integrity is provided by
measured boot, we require profiles to be locked to PCR0 which is not
necessary when PSB is enabled.
…d before the separator

This is a follow up to #442 for
the efi/preinstall package.
efi/preinstall: Permit pre-OS image verification events to be measured before the separator.

This is a follow up to #442 for the efi/preinstall package.
Some AMD systems measure events with unknown event types to PCR7. We
should just permit these.

Fixes: FR-12925
Fixes: #521
…PCR4

We currently consider any pre-OS EV_EFI_BOOT_SERVICES_APPLICATION event
that isn't Absolute to be a sysprep app. However, ignore any events
loaded from flash. This should fix compatibility with some HP systems
which measure multiple EV_EFI_BOOT_SERVICES_APPLICATION events
associated with various firmware components during normal boots.

Fixes: FR-12927
Fixes: #509
As we convert the numerical recovery key into a cryptographic key, we
can be more relaxed about the parsing. Rather than permitting each group
of 5 digits be separated by an optional '-', just permit an arbitrary
number of '-' or whitespace characters intead.

Fixes: FR-11924
Relax recovery key parsing.

As we convert the numerical recovery key into a cryptographic key, we
can be more relaxed about the parsing. Rather than permitting each group
of 5 digits be separated by an optional '-', just permit an arbitrary
number of '-' or whitespace characters instead.

Fixes: FR-11924
…on-hp

efi/preinstall: Permit pre-OS application launches from SPI flash in PCR4.

We currently consider any pre-OS EV_EFI_BOOT_SERVICES_APPLICATION event
that isn't Absolute to be a sysprep app. However, ignore any events
loaded from flash. This should fix compatibility with some HP systems
which measure multiple EV_EFI_BOOT_SERVICES_APPLICATION events
associated with various firmware components during normal boots.

Fixes: FR-12927
Fixes: #509
TPMs based on versions of the refeference library older than v1.38 may
not implement TPM_CAP_AUTH_POLICICES. The preinstall checks currently
rely on this in order to determine whether TPM hierarchies have an
authorization policy set.

The existence of an authorization policy isn't relevant if the
hierarcy's authorization value isn't set, as the empty authorization
value can be used to clear the policy. In the case where the
authorization value is unset, there's no point in checking for the
existence of a policy.

Only check for the existence of an authorization policy for the lockout
hierarchy if it has an authorization value. In this case, the presence
of a policy is presented in the error message as additional information.
We take ownerhip of the lockout hierarchy in
tpm2.Connection.EnsureProvisioned, and for now, this function will clear
any policy before the authorization value is set. A subsequent PR will
set a more appropriate policy based on the requirement to be able to
rotate the authorization value safely during reprovisioning.

Fixes: #408
… mode

Access to the HFSTS registers via the HECI is not possible on systems
that use Intel's High Assurance Platform mode. This means that we can't
check the BootGuard policy. However, the startup ACM mirrors some
BootGuard policy settings to a MSR, so we can check this as a fallback
in this case.

There are some limitations here - it's not possible to check some bits
that indicate the platform has properly transitioned out of
manufacturing mode, and the error enforcement bits of the BootGuard
policy are not available either. However, there are no combinations of
the other policy bits that are duplicated with different error
enforcement bits for any of the recognized profiles, so this doesn't
matter.

Note that the reporter of this issue also noted that turning on HAP mode
resulted in an unexpected EV_EFI_ACTION event being measured to PCR7 on
their system. This doesn't fix that yet because I'm still waiting on a
response from the reporter.
efi/preinstall,tpm2: Don't fail due to lack of TPM_CAP_AUTH_POLICIES.

TPMs based on versions of the refeference library older than v1.38 may
not implement `TPM_CAP_AUTH_POLICICES`. The preinstall checks currently
rely on this in order to determine whether TPM hierarchies have an
authorization policy set.

The existence of an authorization policy isn't relevant if the
hierarcy's authorization value isn't set, as the empty authorization
value can be used to clear the policy. In the case where the
authorization value is unset, there's no point in checking for the
existence of a policy.

Only check for the existence of an authorization policy for the lockout
hierarchy if it has an authorization value. In this case, the presence
of a policy is presented in the error message as additional information.
We take ownerhip of the lockout hierarchy in
`tpm2.Connection.EnsureProvisioned`, and for now, this function will clear
any policy before the authorization value is set. A subsequent PR will
set a more appropriate policy based on the requirement to be able to
rotate the authorization value safely during reprovisioning.

Fixes: #408
…us-msr-in-hap-mode

efi/preinstall: Fall back to checking the BootGuard status MSR in HAP mode.

Access to the HFSTS registers via the HECI is not possible on systems
that use Intel's High Assurance Platform mode. This means that we can't
check the BootGuard policy. However, the startup ACM mirrors some
BootGuard policy settings to a MSR, so we can check this as a fallback
in this case.

There are some limitations here - it's not possible to check some bits
that indicate the platform has properly transitioned out of
manufacturing mode, and the error enforcement bits of the BootGuard
policy are not available either. However, there are no combinations of
the other policy bits that are duplicated with different error
enforcement bits for any of the recognized profiles, so this doesn't
matter.

Note that the reporter of this issue also noted that turning on HAP mode
resulted in an unexpected EV_EFI_ACTION event being measured to PCR7 on
their system. This doesn't fix that yet because I'm still waiting on a
response from the reporter.
* efi/preinstall: Stop relying on BootCurrent

The PCR4 and PCR7 checks were relying on the BootCurrent EFI variable to
identify the EV_EFI_BOOT_SERVICES_APPLICATION event in the TCG log that
corresponds to the launch of the initial OS loader. This is proving
problematic on some systems:
- Whilst some firmware populates a Boot variable containing a load
  option with a path matching the OS loader and sets BootCurrent to
  point to this when booting from removable media, some firmware
  implementations dosn't set a path that matches the OS loader, and this
  path doesn't match any event in the TCG log.
- Some systems don't seem to have BootCurrent set at all when booting
  from removable media.

Instead, assume that the first OS-present
EV_EFI_BOOT_SERVICES_APPLICATION event that isn't Absolute is the
initial OS loader. If it isn't, the PCR4 checks will fail as expected
because os the mismsatched digest. We have to have a special case for
Absolute here because some systems measure this launch as part of
OS-present.

This also makes the checks a bit more permissive where firmware bugs
don't cause problems with generating profiles:
- The PCR4 checks no longer enforce that there can be only one
  EV_OMIT_BOOT_DEVICE_EVENT event.
- The PCR4 checks no longer enforce that there cannot be a EV_EFI_ACTION
  "Calling EFI Application from Boot Option" if there is a
  EV_OMIT_BOOT_DEVICE_EVENT event. We already don't enforce the
  existence of the EV_EFI_ACTION event if there is no
  EV_OMIT_BOOT_DEVICE_EVENT event even though it should be there,
  because we know that some firmware misbehaves here.
- The PCR4 checks no longer gate the reading of the SysPrepOrder and
  SysPrep variables on the BootOptionSupport flags.

Fixes: #517
Fixes: #519

* efi/preinstall: Don't define mock BootCurrent and BootOptionSupport variables in unit tests
Comment thread crypt.go
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOT APPLICABLE

@ernestl ernestl changed the title Review base 3f8b98c2 Changes going from 77bc2457cc76 (2.75.2) to 3f8b98c2db70 (2.74.1) Apr 30, 2026
Comment thread efi/fw_load_handler.go
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RESEALING RELATED

Comment thread efi/preinstall/result.go
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EFFECTS PROFILE USED FOR SEALING

Comment thread efi/preinstall/result.go
val = NoSecureBootPolicyProfileSupport
case "request-partial-dtpm-reset-attack-mitigation":
val = RequestPartialDiscreteTPMResetAttackMitigation
case "require-lock-platform-firmware":
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely this will go to default: instead an error with unrecognized flag "require-lock-platform-firmware"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants